Close the action contract: one registry, no prose fallback (#241 follow-up) - #441
Merged
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Narrowly bounded follow-up to #241, as requested, before the Result API.
What was measured first
A census of 130 pipeline documents, counting only steps without a
tool:key — steps with one route by tool and never touch the action vocabulary:<AUTO>instructionsecho …)My first census was wrong and counted
write/execute/readas prose — they are tool operations in the two-field form. The numbers above are from parsed steps, not rawaction:lines.Among the 167:
generate-text, used six times, had never dispatched. A hyphen slip ofgenerate_text— the same defect class asgenerate_structuredin #437, which is exactly the drift this PR closes.The registry
ActionSpeccarries canonical name, aliases, handler, whether a model or tool is required, required parameters, result schema and v1 membership. Everything else is derived:resolve_action(...).handleris_known_action(...)supported_actionsSUPPORTED_ACTIONScanonical_action(...), applied by the compilerdocs/actions.md, generated + drift-testedThe ten advertised-but-unimplemented names are gone. Anything advertised is now executable, with a test asserting exactly that.
Unknown actions are refused twice
At compile time, and again at dispatch — a caller can build a
Taskand reach execution without going through YAML, so compile-time validation is not the only boundary. Both are tested.<AUTO>...</AUTO>stays supported and is now a declaredActionFamilyrather than an accident of the fallback: an author explicitly asking the model to interpret an instruction is not the same as a typo falling through.echo …and the prose filesystem forms are declared the same way; their regexes moved into the registry and the two now-unused predicate methods are deleted.Three real bugs fell out of writing the tests
generate_structuredlives onModelBasedControlSystem) hit the new raise instead of delegating."file"but not"filesystem". Normalising the alias sent canonical steps past that branch into a default that tried to write to the read-only root.{"action": "retry", "delay": 5.0, ...}— as its error. The reason a step failed was being discarded system-wide, andTask.errorwas not even an exception. Now the original error is recorded with the recovery decision beside it, and a compile failure leads with the actual messages instead ofIssues: 1 errors.Number 3 is the one worth noting for the Result API: structured errors were not merely missing, they were actively overwritten.
Also in here
SyntaxErrorinscripts/utilities/organization_maintenance.py—extend([closed with]. That file could not be imported at all. Found by extending the correctness lint toscripts/.git status --porcelain --untracked-files=all, sincegit diffcannot see newly created ones.legacy-results.txtis excluded as the single expected artifact.tests/test_builtin_actions.pyfolded intotests/test_action_contract.py(53 tests); its assertions were subsumed once the registry became authoritative.Numbers
src+scripts)uv lock --check,git diff --check, ci.yml parseDeliberately not here
Parameter schemas, only required-parameter lists.
ActionSpechas the field, and the validator checks required names, but full JSON Schema per action means deciding the parameter vocabulary for every action — that belongs with the canonical YAML schema work, not smuggled into a closure PR.Result schemas are declared for
generateandgenerate_structuredonly — the two whose envelope shape is actually fixed today. Asserting a schema for handlers that return ad-hoc dicts would mean inventing their contracts here rather than in the Result API, where the per-step envelope gets designed once.